Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: add rootfs image scanning notifications #403

Merged
merged 1 commit into from
Oct 9, 2023
Merged

feat: add rootfs image scanning notifications #403

merged 1 commit into from
Oct 9, 2023

Conversation

ginglis13
Copy link
Contributor

Issue #, if available:

Closes #357

Description of changes:

Amazon ECR offers Enhanced Image Scanning, which we have enabled for the rootfs image used for Finch on Windows. This change implements a stack to automatically notify Finch developers of scan results when the finding is HIGH or CRITICAL in severity (aka CVE score >= 7.0).

An SNS topic is created for the notification, and a Lambda function is wired to EventBridge to listen for events from AWS Inspector of those severities.

Testing done:

Manually deployment and verification of notification email. Emails received from AWS Notifications <[email protected]>. Example text below:

Rootfs Image Security Finding

    Severity: HIGH
    Type: PACKAGE_VULNERABILITY
    Description: Use After Free in GitHub repository vim/vim prior to 9.0.1858.
    Source URL: https://nvd.nist.gov/vuln/detail/CVE-2023-4752

    Status: ACTIVE
    Observed: Sep 7, 2023, 11:35:24 PM

    For more info, view the finding via ARN in the AWS Console: arn:aws:inspector2:us-west-2:<redacted>:finding/<redacted>
  • I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@ginglis13 ginglis13 requested a review from a team September 12, 2023 00:09
pendo324
pendo324 approved these changes Sep 12, 2023
View reviewed changes
Copy link
Member

@pendo324 pendo324 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, lgtm

austinvazquez
austinvazquez reviewed Sep 12, 2023
View reviewed changes
Copy link
Member

@austinvazquez austinvazquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have unit testing for the lambda function. Maybe we could look here for inspiration.

lib/image-scanning-notifications-lambda-handler/main.py Outdated Show resolved Hide resolved
lib/image-scanning-notifications-lambda-handler/main.py Show resolved Hide resolved
lib/image-scanning-notifications-lambda-handler/main.py Outdated Show resolved Hide resolved
weikequ
weikequ approved these changes Sep 13, 2023
View reviewed changes
Copy link
Contributor

@weikequ weikequ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just added a couple of comments. Thanks! :D

lib/event-bridge-scan-notifs-stack.ts Outdated Show resolved Hide resolved
lib/image-scanning-notifications-lambda-handler/main.py Show resolved Hide resolved
@ginglis13 ginglis13 force-pushed the image-scanning-notifs branch 2 times, most recently from fcac304 to 1c1e332 Compare September 20, 2023 16:56
@ginglis13
Copy link
Contributor Author

It would be nice to have unit testing for the lambda function. Maybe we could look here for inspiration.

Thanks for the ref. For now I've opened #412 to track. Since this isn't critical infrastructure, failure in the lambda itself isn't going to be too painful. I'd argue it would be more valuable to ship this and get information on any vulnerabilities in the rootfs image first, and follow up w/ the test. Additionally... given the project is entirely typescript and this is the first introduction of python, would have to do some thinking on structure / potential reworking this to ts. WDYT? @austinvazquez

@austinvazquez
Copy link
Member

It would be nice to have unit testing for the lambda function. Maybe we could look here for inspiration.

Thanks for the ref. For now I've opened #412 to track. Since this isn't critical infrastructure, failure in the lambda itself isn't going to be too painful. I'd argue it would be more valuable to ship this and get information on any vulnerabilities in the rootfs image first, and follow up w/ the test. Additionally... given the project is entirely typescript and this is the first introduction of python, would have to do some thinking on structure / potential reworking this to ts. WDYT? @austinvazquez

Sounds good to me. Thanks for opening the issue for tracking.

@ginglis13 ginglis13 force-pushed the image-scanning-notifs branch from 1c1e332 to e763a37 Compare October 9, 2023 16:39
@ginglis13
feat: add rootfs image scanning notifications
737b4b0 
Amazon ECR offers Enhanced Image Scanning, which we have enabled for the
rootfs image used for Finch on Windows. This change implements a stack
to automatically notify Finch developers of scan results when the
finding is HIGH or CRITICAL in severity (aka CVE score >= 7.0).

An SNS topic is created for the notification, and a Lambda function is
wired to EventBridge to listen for events from AWS Inspector of those
severities.

Signed-off-by: Gavin Inglis <[email protected]>
@ginglis13 ginglis13 force-pushed the image-scanning-notifs branch from e763a37 to 737b4b0 Compare October 9, 2023 16:40
@ginglis13 ginglis13 merged commit c52e4bf into runfinch:main Oct 9, 2023
2 checks passed
@ginglis13 ginglis13 mentioned this pull request Oct 17, 2023
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@austinvazquez austinvazquez austinvazquez approved these changes

@weikequ weikequ weikequ approved these changes

@pendo324 pendo324 pendo324 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add custom stack to enable enhanced image scanning for ECR
4 participants
@ginglis13 @austinvazquez @pendo324 @weikequ